Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add tests for binary values #2099

Merged
merged 18 commits into from
May 13, 2020
Merged

Add tests for binary values #2099

merged 18 commits into from
May 13, 2020

Conversation

nlohmann
Copy link
Owner

This PR adds tests for the binary values introduced in #1662 and fixes some subtle bugs along the way.

Closes #2082.

@nlohmann
Copy link
Owner Author

@OmnipotentEntity Could you please have a look? Maybe I broke something along the way. I am still not 100% sure whether the internal binary type fits as it is right now, but at least now there is better coverage to test some things.

@nlohmann nlohmann self-assigned this May 10, 2020
@nlohmann nlohmann added this to the Release 3.8.0 milestone May 10, 2020
@nlohmann nlohmann added the aspect: binary formats BSON, CBOR, MessagePack, UBJSON label May 10, 2020
@OmnipotentEntity
Copy link
Contributor

I distinctly remember typing up a response to this thread yesterday. I guess I forgot to actually click comment. Whoops.

I clearly didn't understand SAX on my first time through, and I forgot to adjust it as I learned exactly what it was supposed to do, whoops.

Good catch on the binary comparison bug, that one is actually somewhat funny.

I don't quite understand the binary constructor fix, but what I don't understand is why I wrote that code in the first place. I was probably cribbing off of the array code, but that's entirely not necessary for a single valued function.

Tests look good to me. New features are smart additions.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.01%) to 99.841% when pulling 6014419 on issue2082 into 1de30bc on develop.

@nlohmann nlohmann merged commit 9265ca7 into develop May 13, 2020
@nlohmann nlohmann deleted the issue2082 branch May 13, 2020 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aspect: binary formats BSON, CBOR, MessagePack, UBJSON
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing tests for binary values
3 participants